feature/admin-location-ms-access#343
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 590858e772
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
|
|
||
| def register_routes(app): | ||
| from admin.auth_routes import router as admin_auth_router |
There was a problem hiding this comment.
Provide admin.auth_routes before importing it
The new register_routes import expects an admin.auth_routes module, but there is no such file in the repo (only this reference shows up in rg). When register_routes runs at startup, Python will raise ModuleNotFoundError, preventing the app from booting in any environment where routes are registered.
Useful? React with 👍 / 👎.
| # Store original URL to redirect back after login | ||
| original_url = str(request.url_for("admin:index")) | ||
| request.session["auth_redirect"] = original_url | ||
|
|
||
| # Redirect to Authentik login | ||
| return RedirectResponse(url=authentik_authorize_url, status_code=302) | ||
|
|
||
| async def logout(self, request: Request) -> RedirectResponse: | ||
| redirect_uri = str(request.url_for("admin_auth_callback")) |
There was a problem hiding this comment.
Ensure admin_auth_callback route exists for url_for
The login flow now calls request.url_for("admin_auth_callback"), but there is no route with that name defined anywhere in the codebase (the only match is this line). Starlette raises NoMatch when url_for targets an unknown route, so any admin login attempt will error before redirecting to Authentik unless a matching callback route is added.
Useful? React with 👍 / 👎.
Why
This PR addresses the following problem / context:
How
Implementation summary - the following was changed / added / removed:
Notes
Any special considerations, workarounds, or follow-up work to note?